-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow using actual argument name as bind parameter on a single collection #1856
Conversation
@@ -48,6 +51,7 @@ | |||
private final SortedMap<Integer, String> names; | |||
|
|||
private boolean hasParamAnnotation; | |||
private boolean useActualParamName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this variable necessary? No test fails even if I removed it.
If it is necessary for some corner case, there should be a test.
Sorry if I missed something obvious... 😩😪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx! I will confirm my changes again at tomorrow or after.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is necessary for keeping the backward-compatibility when Configuration#useActualParamName
is false
. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Thank you for the update!
It would be simpler to store the value of config.isUseActualParamName()
in a final field. i.e.
private final boolean useActualParamName;
public ParamNameResolver(Configuration config, Method method) {
useActualParamName = config.isUseActualParamName();
...
Or, keep the Configuration
instance and reference the value directly.
return wrapToMapIfCollection(
value, config.isUseActualParamName() ? names.get(0) : null);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! I've applied a your comment via ce14cec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Now we have all the parameter name resolution related logic in one class. :)
Thank you, @kazuki43zoo !
Fixes gh-1237
I've made available access to a single collection and array as bind parameter using actual parameter name.